Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only sleep if there are no tasks #91

Merged
merged 2 commits into from
Aug 12, 2024

Conversation

mgax
Copy link
Contributor

@mgax mgax commented Jul 31, 2024

I was running the DB worker on a queue full of tasks, and was wondering why it was only processing one task per second. It's bad form to sleep when there is work to do.

@RealOrangeOne
Copy link
Owner

I'm not sure how I feel about this, as it changes the definition of "interval". It could be surprising to some people. This change increases throughput, but also adds DB load.

It's also arguably a breaking change.

@mgax
Copy link
Contributor Author

mgax commented Aug 2, 2024

Fair points; on the other hand, I don't think it's common/expected for a task queue to throttle its throughput to keep resource use under control. If a user really wants this, they could add a time.sleep() themselves, either in the task, or in a post-execute hook.

@RealOrangeOne
Copy link
Owner

Change looks good now! Although I'm a little concerned about it being a breaking change. The help text for --interval mentions checking for new tasks. I can't think of a better name than "interval", but the text should be updated to match its new use.

@mgax
Copy link
Contributor Author

mgax commented Aug 2, 2024

Although I'm a little concerned about it being a breaking change.

That's a good argument for the next release being 0.4.0 :)

but the text should be updated to match its new use.

How about The interval (in seconds) to wait, when there are no tasks in the queue, before checking for tasks again?

@RealOrangeOne
Copy link
Owner

The interval (in seconds) to wait, when there are no tasks in the queue, before checking for tasks again?

Works for me!

the next release being 0.4.0

It probably will be. I've got some other bigger changes in the work which warrant a larger release bump.

@mgax
Copy link
Contributor Author

mgax commented Aug 2, 2024

Alright; I've updated the sleep condition and the help text for the CLI.

@RealOrangeOne RealOrangeOne merged commit fa4feaa into RealOrangeOne:master Aug 12, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants